fix(evm): ten bugs from a self-review pass over the network driver - #2231

The finality watcher polled StatusByAnchor until either the anchor appeared
or the timeout expired, and on timeout it always reported Invalid. If the
chain was unreachable for the whole window, every poll errored and got
skipped, so the loop reached the timeout having never actually observed the
ledger, and reported Invalid anyway. The shared ttx listener maps Invalid
straight to a deleted transaction, so a connectivity outage on the reading
side could make a transaction that actually committed look failed, and its
tokens would be dropped from local bookkeeping.

The watcher now tracks whether any poll in the window actually reached the
chain, valid or not. Only then does an absent anchor at the timeout mean
Invalid. If every attempt errored, it reports OnError instead, the same
signal the interface already defines for "the finality event could not be
delivered." The transaction stays Pending rather than being marked deleted,
and the driver's existing recovery sweep picks it up again later with a
fresh read.

Signed-off-by: atharrva01 <atharvaborade568@gmail.com>

The default block tag is finalized, and reading at that tag has a real
time-to-finality of roughly 13 minutes (design §7.2). The default finality
timeout was 5 minutes, so a deployment running on defaults alone would time
out on every single transaction, valid or not, and report it Invalid before
the chain could ever finalize it. The design already documents this exact
constraint (§7.5: "any deployment must configure finality.timeout above ...
the chain's finality"), but nothing enforced it.

DefaultFinalityTimeout is now 20 minutes, with real margin over the ~13
minute floor rather than sitting at its edge. Validate also now rejects a
finalized-tag configuration whose timeout is shorter than that floor, so a
deployer who explicitly sets an unsafe combination gets a startup error
instead of every transaction silently failing later. The floor applies only
to the finalized tag; safe and latest resolve on their own, faster
schedules and are not validated here.

Signed-off-by: atharrva01 <atharvaborade568@gmail.com>

The watcher recorded a new version as seen before calling the handler that
actually applies it, and the handler had no way to report failure at all
(UpdateHandler returned nothing). If applying a version failed for any
reason, the watcher had already moved past it: the next poll only looks at
what changed since the last seen version, so a failed reload was silently
never retried, and the node kept serving stale public parameters with
nothing left to notice the gap.

UpdateHandler now returns an error, and the watcher only advances past a
version once its handler actually succeeds; a failure is logged and the
same version is retried on the next poll. applyPublicParams collects and
returns the combined error of every TMS that failed to update, so a
partial failure is visible to the watcher rather than swallowed. Retrying
the whole batch is safe: updating a TMS with parameters it already holds
is a no-op, so a TMS that already succeeded is not disturbed by covering
it again.

Covered at the watcher, where the actual defect lived: a new test drives a
handler that fails twice then succeeds and asserts the same version is
retried rather than skipped, and that seen only advances on the eventual
success.

Signed-off-by: atharrva01 <atharvaborade568@gmail.com>

ChainProvider.PublicParams read the parameter bytes and the version as two
separate, unsynchronised calls. An endorsed setup delta landing between
them could tear the pair: bytes from before the update, version from
after, or the reverse. The contract checks both fields together against
what it currently holds and reverts StalePublicParams on a mismatch, so a
torn read here did not corrupt state, but it turned a purely local race in
this function into a doomed, gas-spending transaction the contract was
always going to reject.

The version is now read before and after the bytes, and the whole read is
retried if it moved: version and bytes only ever change together, in the
same transaction, so two matching reads bracketing the bytes read is proof
nothing landed in between. The retry is bounded (three attempts) so a
pathological chain that never settles fails with an error instead of
spinning.

TestWatcherSurvivesAFailedRead needed a related fix: it modelled the
version as the raw RPC call count, which does not hold once PublicParams
reads the version twice per attempt, two call counts a few lines apart
would themselves look torn. Rewritten to use the same stable chain-state
double the other watcher tests already use.

Signed-off-by: atharrva01 <atharvaborade568@gmail.com>

SetupPublicParams checked only Network.endorsement, the field tests inject
a stub into directly. In production nothing ever sets that field: the
driver wires the per-TMS endorsement factory through endorsementFor
instead, keyed by an already-built management service. SetupPublicParams
never has one of those to hand it, that is the entire reason it takes a
bare TMSID rather than a management service, so it could never reach the
factory at all. Every call failed with "no endorsement service configured"
on any real deployment, and first-time setup of a namespace, the one thing
this method exists to make possible, could not work.

Nothing caught this: the shared ppsetup view exercises the real production
path, but the EVM integration suite bootstraps and updates parameters
through its own harness-side submitter instead, bypassing this method
entirely, so the gap was invisible to every existing test.

Network now also carries endorsementForID, a TMSID-keyed counterpart to
endorsementFor, and SetupPublicParams resolves through that instead. The
driver installs it in installEndorsement next to the existing factory:
both ultimately call the same per-TMS ServiceFactory.ForTMS, one entered
from a management service, the other from the id alone.

New tests cover what nothing did before: SetupPublicParams resolving
through the id-based factory and reaching it with the requested TMS, and a
failed endorsement collection not broadcasting.

Signed-off-by: atharrva01 <atharvaborade568@gmail.com>

…nonce allocation

NonceManager split allocation and recovery into two separate critical
sections: Next handed out a nonce and released the lock, and a failed
Submit later called Reset, which walked the whole sequence back to
whatever eth_getTransactionCount(pending) currently showed. That call
only reflects transactions the node has actually seen. A different,
concurrent Submit that had already allocated a higher nonce but not yet
reached SendRawTransaction was invisible to it, and Reset would hand that
same nonce to a third caller, producing two transactions racing for one
nonce.

Every path that called Reset already treated its own failure as certain
proof the transaction never reached the chain: gas estimation and fee
suggestion are read-only, signing is local, and a rejected broadcast is
documented as never judged by the chain. So walking back to the chain's
view was never actually necessary, it just happened to be how the
recovery was implemented, and that implementation was what raced.

NonceManager.Next and Reset are replaced by WithNonce, which holds the
lock for the whole allocate-and-use step. The sequence advances only if
the callback succeeds; on failure the nonce is simply left where it was,
with no round trip to the chain, and nothing else could have been mid
allocation while the callback ran. Submitter.Submit now runs its entire
body inside that callback.

New tests cover the failure path directly (a failed attempt does not
advance the sequence and needs no re-sync) and drive many goroutines with
a mix of successes and failures to check every successful attempt still
gets a distinct nonce.

Signed-off-by: atharrva01 <atharvaborade568@gmail.com>

…discarding it silently

Driver.New runs once per (network, channel) the node is configured for,
and installEndorsement, called from it, builds a fresh per-network
ServiceFactory and tries to register this node's endorsement responder
every time. That registration used a sync.Once scoped to the whole
Driver, not to a network, so on a node configured to endorse for more
than one EVM network, only the first network's registration ever ran.
Every later network's factory, key and allowlist were silently discarded,
with nothing logged to say so.

The reason a straight per-network fix is not possible: FSC routes an
incoming session to a responder by the initiating view's Go type alone,
with no notion of "this responder, but only for network X". Only one
Responder can ever be registered for endorsement.Initiator across a
process's lifetime, so whichever network's factory happens to win the
race is baked into it permanently, EIP-712 domain, chain client and all.
Routing a second network's requests through it would not fail cleanly,
it would validate against the right TMS but sign and read against the
wrong chain.

registerEndorser now tracks which network it registered for and refuses,
loudly, when a different network also wants to endorse: an operator gets
a clear error naming both networks instead of a request that silently
never gets answered. The same network registering twice (a network
rebuilt over a node's life) and a network that never wanted to endorse in
the first place both remain unaffected.

Signed-off-by: atharrva01 <atharvaborade568@gmail.com>

…t registration

Two comments, on the Allowlist field and on Authorizer itself, promised
that an empty allowlist would default to "the TMS network's nodes,
resolved at config load in Week 5". That resolution was never built.
Authorizer.NewAuthorizer is deliberately fail-closed and rejects an empty
allowlist outright, which is the right call for authorization, but
nothing upstream of it ever supplied the promised default, so an operator
who left Allowlist unset trusting the documented behavior got a node that
came up looking healthy and silently never registered as an endorser, the
failure logged as an error easy to miss during wiring rather than
surfaced as the startup failure it should have been.

Validate now rejects an endorser.enabled configuration with no allowlist,
matching this file's own stated philosophy that a bad configuration
should be a startup error, not a surprise later. Both comments are
corrected to describe the actual, intentional fail-closed behavior
instead of a fallback that does not exist.

The integration harness is unaffected: it already builds the allowlist
itself from every node in the TMS rather than relying on the driver to
do it, which is what the documented default was supposed to be doing.

Signed-off-by: atharrva01 <atharvaborade568@gmail.com>

…gree

Broadcast checked only that an envelope carried a delta at all, never
that the envelope's own anchor and the anchor baked into that delta
actually named the same transaction. Under the normal flow they always
agree, since RequestApproval and SetupPublicParams both derive the
envelope's anchor and the delta's anchor from the same value, but
Broadcast has no way to know how the envelope it was actually handed was
built.

A mismatch here is not just a theoretical validation gap. The chain only
ever looks at the delta's anchor: that is what applyStateDelta checks for
replay, what the digest covers, what StateCommitted is emitted for. The
local side tracks the transaction by the envelope's anchor instead,
finality listeners and the ttx store are keyed on it. If the two ever
diverged, the transaction would apply and commit on chain under one
anchor while everything local kept waiting on a different one, and after
the finality timeout wrongly report a transaction that actually succeeded
as failed, the same failure shape HIGH #1 fixed, just reachable through a
construction bug instead of a chain-read one.

Broadcast now parses the envelope's anchor and compares it against the
delta's before spending any gas, and refuses if they disagree.

Signed-off-by: atharrva01 <atharvaborade568@gmail.com>

…ted tags

FinalityConfig.BlockTag's own comment said state is read at finalized
or safe, but Validate has accepted latest as a third legal value since
it was introduced for the local, instant-mining test harness. The
comment now names all three and repeats, next to the field itself,
what BlockTagLatest's own doc comment already says: it carries no
reorg protection and is only appropriate for a local chain.

No behavior changes; latest was already accepted before this commit.

Signed-off-by: atharrva01 <atharvaborade568@gmail.com>